-
Notifications
You must be signed in to change notification settings - Fork 51
Symbol visibility: Hidden by default #1793
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
set_target_properties(openPMD PROPERTIES | ||
CXX_VISIBILITY_PRESET hidden | ||
VISIBILITY_INLINES_HIDDEN ON | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We did this for the old ADIOS1 libraries if you like to take a look:
https://github.com/openPMD/openPMD-api/blob/0.14.5/CMakeLists.txt#L555-L571
|
||
# symbol hiding | ||
if(openPMD_HAVE_SYMBOLHIDING) | ||
set_target_properties(openPMD PROPERTIES |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not work for C++ users, unless you explicitly export all our public C++ APIs.
Already, we should not be exporting C++ stdlib symbols, I think. I think that mixing different C++ stdlibs generally does not work in the same process, just how the dynamic loader works and populates symbols. But this is not needed in most cases, e.g., on HPC one does instead compile with a consistent set of compilers/modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already, we should not be exporting C++ stdlib symbols, I think
Right, I see that this PR affects only symbols for our IO backends, which can be theoretically linked fully internally. I'll close.
This revisits #165 as an attempt to fix software stack issues such as #1792 where binary-incompatible versions of e.g. the C++ stdlib are used by openPMD-api and other packages.
TODO: